Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add autoware_node and autoware_test node #113

Merged
merged 11 commits into from
Dec 10, 2024
Merged

feat: add autoware_node and autoware_test node #113

merged 11 commits into from
Dec 10, 2024

Conversation

xmfcx
Copy link
Contributor

@xmfcx xmfcx commented Dec 7, 2024 •
edited
Loading

Description

  • Split from feat: add autoware_control_center and autoware_node #84 to simplify the reviewing process.
  • autoware_node inherits from the rclcpp::LifecycleNode and provides the base class for the remaining Autoware nodes.
  • autoware_test_node is a reference implementation of an expected new autoware node, inheriting from the autoware_node.
    • And provides a launch file to run it.

Helpful links

ROS 2 design and demos

From Nav 2

Example implementations:

How was this PR tested?

See the README.md of the autoware_test_node from this PR.

And autoware_node includes a init-shutdown colcon test, CI will test it.

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

@xmfcx xmfcx self-assigned this Dec 7, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 7, 2024 •
edited
Loading

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@codecov Codecov
Copy link

codecov bot commented Dec 7, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 55.17241% with 13 lines in your changes missing coverage. Please review.

Project coverage is 73.25%. Comparing base (a61009d) to head (de30f05).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ommon/autoware_node/test/test_an_init_shutdown.cpp 66.66% 1 Missing and 4 partials ⚠️
demos/autoware_test_node/src/test_node.cpp 0.00% 5 Missing ⚠️
common/autoware_node/src/node.cpp 75.00% 0 Missing and 2 partials ⚠️
...ommon/autoware_node/include/autoware/node/node.hpp 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #113      +/-   ##
==========================================
- Coverage   76.92%   73.25%   -3.67%     
==========================================
  Files           6       10       +4     
  Lines         143      172      +29     
  Branches       55       68      +13     
==========================================
+ Hits          110      126      +16     
- Misses         12       19       +7     
- Partials       21       27       +6
Flag Coverage Δ *Carryforward flag
differential 55.17% <55.17%> (?)
total 76.92% <ø> (ø) Carriedforward from ea6f047

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xmfcx
Copy link
Contributor Author

xmfcx commented Dec 7, 2024 

/home/runner/work/autoware.core/autoware.core/common/autoware_node/src/node.cpp:15:0: information: Include file: "autoware/node/node.hpp" not found. [missingInclude]
#include "autoware/node/node.hpp"
^
/home/runner/work/autoware.core/autoware.core/common/autoware_test_node/src/test_node.cpp:30:0: information: Include file: "rclcpp_components/register_node_macro.hpp" not found. [missingInclude]
#include "rclcpp_components/register_node_macro.hpp"
^

cppcheck-differential failed with the messages above.
I don't know why it cannot find those files, it works and files are present.

@xmfcx
Copy link
Contributor Author

xmfcx commented Dec 7, 2024

@youtalk @mitsudome-r ready for review ✅

youtalk
youtalk reviewed Dec 10, 2024
View reviewed changes
Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the autoware_test_node package is very helpful. But since it’s a demo package, it feels strange for it to be in the common directory. How about moving it to a directory with a name like demos?

common/autoware_node/include/autoware/node/node.hpp Show resolved Hide resolved
@xmfcx
Copy link
Contributor Author

xmfcx commented Dec 10, 2024

I think the autoware_test_node package is very helpful. But since it’s a demo package, it feels strange for it to be in the common directory. How about moving it to a directory with a name like demos?

Moved it to demos folder ✅

xmfcx added 3 commits December 10, 2024 14:00
@xmfcx
fix pre-commit
9efb556 
Signed-off-by: M. Fatih Cırıt <[email protected]>
@xmfcx
remove node namespace
7cbb780 
Signed-off-by: M. Fatih Cırıt <[email protected]>
@xmfcx
Revert "remove node namespace"
ea6f047 
This reverts commit 7cbb780.
Signed-off-by: M. Fatih Cırıt <[email protected]>
@xmfcx
clean up the test
de30f05 
Signed-off-by: M. Fatih Cırıt <[email protected]>
youtalk
youtalk approved these changes Dec 10, 2024
View reviewed changes
Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@xmfcx
Copy link
Contributor Author

xmfcx commented Dec 10, 2024

Thank you for the review!

@xmfcx xmfcx merged commit 1bba5d3 into autowarefoundation:main Dec 10, 2024
27 of 29 checks passed
@xmfcx xmfcx deleted the feat/add-acc-an branch December 10, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youtalk youtalk youtalk approved these changes

@mitsudome-r mitsudome-r Awaiting requested review from mitsudome-r

Assignees

@xmfcx xmfcx

Labels
tag:run-build-and-test-differential
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xmfcx @youtalk